-
-
Notifications
You must be signed in to change notification settings - Fork 735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow 'today' modifier to override default #279
Conversation
src/DayPicker.js
Outdated
let dayModifiers = []; | ||
if (DateUtils.isSameDay(day, new Date())) { | ||
dayModifiers.push(this.props.classNames.today); | ||
const { today } = this.props.classNames; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maxdubrinsky could you change the name of this var to todayModifier
? So it's easier to understand what it is... thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing.
src/DayPicker.js
Outdated
const { today } = this.props.classNames; | ||
const propModifiers = Helpers.getModifiersFromProps(this.props); | ||
const dayModifiers = Helpers.getModifiersForDay(day, propModifiers); | ||
if (DateUtils.isSameDay(day, new Date()) && !propModifiers.hasOwnProperty(today)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't use hasOwnProperty
this way or the lint will complain:
- propModifiers.hasOwnProperty(today)
+ Object.prototype.hasOwnProperty.call(propModifiers, today)
thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Just about to ask how you guys wanted that handled syntacticly. I would normally use in
, but there are a couple of reasons not to for performance reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even if uglier I'd just use Object.prototype.hasOwnProperty
:)
Good idea, thanks – do you feel to add a test as well? 👍 |
Weird it didn't catch the lint, did you run the folowing?
|
I did, but apparently I added that after I ran |
@maxdubrinsky are you working on this during the next... hours 😊? I'd like to release an update soon 🚀 |
Sorry for the delay, I will be able to push an update in the next hour or so. |
No problem, thanks for your effort ❤️ |
Awesome thanks! |
Thanks for being so prompt! Saved me a bunch of time and headache |
Thanks, published as v5.2.0 🎉 |
The product I am working on allows users to specify a timezone other than that of their browsers, and in doing so might cause what the day picker component thinks
today
is to be different from what the user expects. This change simply allows me to override thetoday
modifier with a custom function that could use other references, such asmoment-timezone
objects.This was run with
test
,cover
, andlint
, but if there are other modifications necessary, please let me know.Thanks, and I welcome your feedback!